-
-
Notifications
You must be signed in to change notification settings - Fork 694
fix: support Connection header with connection-specific header names per RFC 7230 #4775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…per RFC 7230 Per RFC 7230 Section 6.1, the Connection header can contain a comma-separated list of connection option tokens (header names) that should be removed by proxies before forwarding the message. Previously, undici only allowed 'close' or 'keep-alive' as Connection header values. This change allows any valid HTTP token as a connection option, enabling RFC-compliant requests like: Connection: X-Custom-Header Connection: close, X-Custom-Header Fixes: #4774 Signed-off-by: Matteo Collina <hello@matteocollina.com>
lib/core/request.js
Outdated
| } | ||
|
|
||
| if (value === 'close') { | ||
| const tokens = value.split(',').map(t => t.trim().toLowerCase()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are iterating over them twice, can we do it at one pass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could actually lowercase before split.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Refactored to lowercase before split and combined validation + close-check in a single loop.
- Lowercase before split as suggested - Combine validation and close-check in single loop Signed-off-by: Matteo Collina <hello@matteocollina.com>
The previous test used 'asd' as an invalid connection header value, but 'asd' is a valid HTTP token per RFC 7230. Updated to use a truly invalid value (token with spaces) instead. Signed-off-by: Matteo Collina <hello@matteocollina.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4775 +/- ##
==========================================
- Coverage 93.27% 93.26% -0.02%
==========================================
Files 109 109
Lines 34001 34009 +8
==========================================
+ Hits 31713 31717 +4
- Misses 2288 2292 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
closeorkeep-alivewere accepted, now any valid HTTP token is allowedrequest.reset = trueifcloseis present in the token listTest plan
closeandkeep-alivevaluesFixes #4774